chore(generator): delegate request-id setup to public request by hebaalazzeh · Pull Request #17739 · googleapis/google-cloud-python

hebaalazzeh · 2026-07-16T15:16:46Z

Updates generator templates and goldens to import public requests from google-api-core gapic_v1 and call setup_request_id helper. Removes duplicate setup_request_id test logic from generated client unit tests.

Temporarily pointing to this PR then will revert once it is merged/released:

Associated / Dependent API Core PR: feat(api_core): add request-id auto-population logic to gapic_v1 public helpers #17738

gemini-code-assist

Code Review

This pull request refactors the request ID setup logic in the generated client templates by delegating it to google.api_core.gapic_v1.method_helpers.setup_request_id instead of maintaining an inline implementation. As a result, the redundant unit tests for _setup_request_id have been removed from both the templates and the golden integration tests. There are no review comments, so I have no feedback to provide.

daniel-sanche · 2026-07-16T18:12:59Z

This implementation makes sense if we plan to bump up the api_core requirement Before merging, we need to:

get the changes in feat(api_core): add request-id auto-population logic to gapic_v1 public helpers #17738 released
update the setup.py to reference the new api_core version floor
regenerate goldens

There's also a chance we will want to keep a fallback implementation to keep supporting old api_core versions

…lic helpers Introduces method_helpers module containing setup_request_id helper. Exposes the module as public in gapic_v1.

…t and improve docstring

…lic helpers Introduces method_helpers module containing setup_request_id helper. Exposes the module as public in gapic_v1.

Updates generator templates and goldens to import public method_helpers from google-api-core gapic_v1 and call setup_request_id helper. Removes duplicate setup_request_id test logic from generated client unit tests.

…ator

…t and improve docstring

…per directly

…centralization-request-id

… import

…lient.py to use setup_request_id

… TODO issue

… _compat.py

…vent coverage drop in showcase

…populated_fields

…coverage pragma

…eader

ohmayr · 2026-07-21T20:31:45Z

+                    setattr(request, field_name, request_id_val)
+            except (AttributeError, ValueError):
+                # Proto-plus messages or other objects
+                if not getattr(request, field_name, None):


Is this line consistent to what we have in the requests.py in api-core?

ohmayr · 2026-07-21T20:33:43Z

+            request (Union[google.protobuf.message.Message, dict]): The request object.
+            field_name (str): The name of the field to populate.
+            is_proto3_optional (bool): Whether the field is proto3 optional.
+        """


do we need the check below (similar to what we have in api-core)

if request is None: return

ohmayr · 2026-07-21T20:34:27Z

 from google.api_core.client_options import ClientOptions
 from google.api_core import exceptions as core_exceptions
 from google.api_core import gapic_v1
+{% if has_auto_populated_fields.value %}


Why do we only generate this file when has_auto_populated_fields is true?

ohmayr · 2026-07-21T20:35:52Z

+except ImportError:  # pragma: NO COVER
+    # TODO(https://github.com/googleapis/google-cloud-python/issues/17813): Remove this fallback when google-api-core >= 2.26.0 is the minimum required version.
+    def setup_request_id(request, field_name: str, is_proto3_optional: bool):  # pragma: NO COVER


Why do we need these pragma comments?

daniel-sanche · 2026-07-21T20:27:24Z

            request (Union[google.protobuf.message.Message, dict]): The request object.
            field_name (str): The name of the field to populate.
            is_proto3_optional (bool): Whether the field is proto3 optional.
        """


the diff here looks pretty broken

daniel-sanche · 2026-07-21T20:28:20Z

+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+


we should have a file-level docstring explaining this file

daniel-sanche · 2026-07-21T20:34:00Z

        )

-        self._client._setup_request_id(request, 'request_id', False)
+        setup_request_id(request, 'request_id', False)


doesn't this need to be imported?

daniel-sanche · 2026-07-21T20:35:21Z

+
+try:
+    from google.api_core.gapic_v1.requests import setup_request_id  # type: ignore
+except ImportError:  # pragma: NO COVER


do we need the NO COVER here? It feels like it could be risky if this code isn't covered. We should have some tests that cover the lower bound for api-core, which would hit this

I left a similar comment above i.e. we'll probably miss coverage and adding # pragma: NO COVER is not the right way to silence coverage here.

However, I believe _compat.py will only generate static / helper code. Ideally, all the code in here should be identical to what we add to api-core (we should have a strict contract about this somewhere in the file). Given that, generating these tests don't really provide any value (if these tests are already well tested in api-core).

So it shouldn't be risky.

…_core requests.py

daniel-sanche · 2026-07-21T20:52:16Z

        if not opts.metadata and template_name.endswith("gapic_metadata.json.j2"):
            return answer

+        # Only render _compat.py.j2 if the API schema has auto_populated_fields


Can you explain this a bit more? What are auto_populated_fields?

gemini-code-assist Bot reviewed Jul 16, 2026

View reviewed changes

hebaalazzeh changed the base branch from main to feat/gapic-centralization-api-core-request-id July 16, 2026 15:20

hebaalazzeh self-assigned this Jul 16, 2026

hebaalazzeh closed this Jul 16, 2026

hebaalazzeh reopened this Jul 16, 2026

hebaalazzeh requested a review from daniel-sanche July 16, 2026 15:49

hebaalazzeh force-pushed the feat/gapic-generator-centralization-request-id branch from f037044 to f25d768 Compare July 16, 2026 16:10

hebaalazzeh force-pushed the feat/gapic-centralization-api-core-request-id branch from a0bff77 to 613b392 Compare July 16, 2026 16:18

hebaalazzeh force-pushed the feat/gapic-generator-centralization-request-id branch from f25d768 to 71b2f4e Compare July 16, 2026 16:20

hebaalazzeh force-pushed the feat/gapic-centralization-api-core-request-id branch 3 times, most recently from e02ee4b to db040bb Compare July 16, 2026 16:28

daniel-sanche added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 16, 2026

hebaalazzeh force-pushed the feat/gapic-generator-centralization-request-id branch from 71b2f4e to a8cc8d8 Compare July 16, 2026 18:11

hebaalazzeh changed the title ~~feat: delegate request-id setup to public method_helpers~~ chore(generator): delegate request-id setup to public request Jul 16, 2026

daniel-sanche reviewed Jul 16, 2026

View reviewed changes

Comment thread ...apic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 Outdated

hebaalazzeh added 3 commits July 16, 2026 23:19

feat(api-core): move request-id auto-population logic to gapic_v1 pub…

79f3668

…lic helpers Introduces method_helpers module containing setup_request_id helper. Exposes the module as public in gapic_v1.

refactor(api-core): rename gapic_v1.method_helpers to gapic_v1.reques…

62fbcc8

…t and improve docstring

chore: address PR review comments for gapic centralization request ID

20adea6

hebaalazzeh force-pushed the feat/gapic-centralization-api-core-request-id branch from 90602cc to 76ac456 Compare July 16, 2026 23:21

hebaalazzeh added 7 commits July 16, 2026 23:25

feat(api-core): move request-id auto-population logic to gapic_v1 pub…

f03ac7c

…lic helpers Introduces method_helpers module containing setup_request_id helper. Exposes the module as public in gapic_v1.

feat: delegate request-id setup to public method_helpers

2cc27b5

Updates generator templates and goldens to import public method_helpers from google-api-core gapic_v1 and call setup_request_id helper. Removes duplicate setup_request_id test logic from generated client unit tests.

refactor: use request module instead of method_helpers in gapic-gener…

9c2121c

…ator

refactor(api-core): rename gapic_v1.method_helpers to gapic_v1.reques…

dbc7307

…t and improve docstring

chore: address PR review comments for gapic centralization request ID

5f1b721

chore: remove private _setup_request_id and call setup_request_id hel…

99cee8e

…per directly

test(api-core): update UUID regex in test_request.py

4ff1910

hebaalazzeh force-pushed the feat/gapic-centralization-api-core-request-id branch from 8e31d83 to 20adea6 Compare July 16, 2026 23:42

refactor(api-core): rename request.py to requests.py for consistency

77f97c8

hebaalazzeh added 5 commits July 21, 2026 15:06

Merge remote-tracking branch 'origin/main' into feat/gapic-generator-…

5af5222

…centralization-request-id

Merge remote-tracking branch 'origin/main' into feat/gapic-generator-…

6ef6dd4

…centralization-request-id

chore(generator): update golden client.py files with setup_request_id…

61aaabb

… import

fix(generator): add utf-8 coding header to _compat.py.j2 template

0b2ceb2

chore(generator): update storagebatchoperations client.py and async_c…

3204cea

…lient.py to use setup_request_id

ohmayr reviewed Jul 21, 2026

View reviewed changes

hebaalazzeh added 7 commits July 21, 2026 18:35

refactor(generator): optimize setup_request_id uuid creation and link…

3e6b78c

… TODO issue

docs(generator): update TODO issue link to #17813 in _compat.py

1e6b56e

chore: trigger Kokoro system tests build rerun

ea8d263

fix(generator): add pragma no cover to setup_request_id definition in…

8e9f505

… _compat.py

fix(generator): add module level pragma no cover to _compat.py to pre…

142d337

…vent coverage drop in showcase

fix(generator): only generate _compat.py if API schema contains auto_…

e5f8e13

…populated_fields

fix(generator): fix proto-plus field check in setup_request_id fallback

905d2b1

ohmayr reviewed Jul 21, 2026

View reviewed changes

Comment thread packages/gapic-generator/tests/integration/goldens/eventarc/google/cloud/eventarc_v1/_compat.py Outdated

hebaalazzeh added 2 commits July 21, 2026 19:21

fix(generator): revert goldens without auto_populated_fields and fix …

7b7668c

…coverage pragma

fix(generator): remove encoding and pragma no cover from _compat.py h…

536a20e

…eader

ohmayr reviewed Jul 21, 2026

View reviewed changes

Comment thread ...ges/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2

fix(generator): remove unused get_uuid4_re macro

a8105ff

hebaalazzeh force-pushed the feat/gapic-generator-centralization-request-id branch 2 times, most recently from 20e4bea to a8105ff Compare July 21, 2026 20:23

ohmayr reviewed Jul 21, 2026

View reviewed changes

daniel-sanche requested changes Jul 21, 2026

View reviewed changes

fix(generator): update fallback proto-plus check to exactly match api…

e243bbd

…_core requests.py

hebaalazzeh force-pushed the feat/gapic-generator-centralization-request-id branch from 4659080 to e243bbd Compare July 21, 2026 20:38

fix(generator): rename requests import to request to match api-core

926eb3c

daniel-sanche reviewed Jul 21, 2026

View reviewed changes

fix(generator): address PR review comments in _compat.py.j2

cd6b66e

Uh oh!

Conversation

hebaalazzeh commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

daniel-sanche commented Jul 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hebaalazzeh commented Jul 16, 2026 •

edited

Loading